Skip to content

0.1.18: DDP fix + API hardening + repo polish + relax torch pin#29

Merged
zhen-he merged 11 commits into
mainfrom
fix/ddp-mosaic-sampler
May 2, 2026
Merged

0.1.18: DDP fix + API hardening + repo polish + relax torch pin#29
zhen-he merged 11 commits into
mainfrom
fix/ddp-mosaic-sampler

Conversation

@zhen-he

@zhen-he zhen-he commented May 2, 2026

Copy link
Copy Markdown
Collaborator

8 commits, target 0.1.18 release.

  • DDP sampler fix (commit 857e8f5, headline of this branch)
  • API hardening: encoder, optimizer, batch_names, downloads (408473b)
  • Docs harmonisation: GPU config block in basics demos (9a95472)
  • Release prep: README polish, single-source version, release notes (6270846)
  • README slim down + emoji removal + bibtex fix (714c8b6, 6a3c00f)
  • Logo width restored to 900px (33965ac)
  • CI matrix 3.10/3.11/3.12 + py.typed + project URLs + CI badge (db3f9ff)

zhen-he added 11 commits April 30, 2026 08:12
… DDP

Two related issues caused DDP training on mosaic data (non-uniform
per-sub-batch modality combinations) to either run with no throughput
gain or hang on an NCCL all-reduce. Both are fixed together because
fixing only the first would silently turn previously working slow DDP
runs into hanging fast DDP runs.

(1) Sampler selection (model.py):

The default sampler_type='auto' fell through to MultiBatchSampler, a
rank-agnostic sampler. Under DDP, every rank then iterated the same
N batches, giving 1x throughput at NxGPU cost. Fix: 'auto' now picks
MyDistributedSampler when torch.distributed is initialized.

(2) MyDistributedSampler.__iter__ (data.py):

The sampler called random.shuffle() on the global Python random
module, so each post-fork DDP rank used its own random state. Two
consequences:
  - The dataset-visit order diverged across ranks. With non-uniform
    per-sub-batch modality combinations, ranks ran the encoder over
    different modality subsets at the same step, producing different
    sets of "unused" parameters per rank. With find_unused_parameters
    =False (Lightning default), DDP all-reduce expects identical grad
    buffers per step and hung waiting for collectives that never
    arrived; NCCL watchdog killed the run after the timeout.
  - random.shuffle(indices) mutated self.all_indices in place, so
    successive epochs cumulatively shuffled an already-shuffled list.

Fix: introduce two seeded random.Random instances per epoch — one
shared across ranks for the dataset-visit order, one rank-specific
for the within-dataset shuffle — and copy self.all_indices[idx]
before shuffling. Also call super().__init__() so self.epoch is
initialised and Lightning's automatic sampler.set_epoch(epoch) takes
effect each epoch.

Behavior change to call out:
- Single-GPU users (MultiBatchSampler) are unaffected.
- DDP users who passed sampler_type='ddp' explicitly: same fix path
  applies; sampling order changes, so prior seeded DDP runs will not
  reproduce numerically.
- DDP users on the implicit 'auto' default: now actually shard data
  across ranks; expect a roughly NxGPU throughput improvement.

Release notes updated in docs/source/release.md.
Five user-visible bugs that survived 0.1.17, plus a regression test
file to pin them down.

* Encoder.forward: replace in-place ``data[m] *= mask`` with out-of-place
  ``data[m] = data[m] * mask``. Mathematically equivalent (mask is a
  binary 0/1 modality-presence indicator and calc_recon_loss already
  multiplies the loss by the same mask), but the in-place form mutated
  the caller's batch dict for any modality without a ``trsf_before_enc_*``
  transform. Defensive — makes the encoder safe to re-call on the same
  batch (e.g. predict's mod_latent / translate paths).
* configure_optimizers: read ``load_optimizer_state`` via ``getattr(...,
  False)``. The attribute is only set on the class by
  configure_data_from_dir / configure_data_from_mdata / load_checkpoint,
  so users entering through the simpler configure_data API previously
  crashed on first trainer.fit with AttributeError.
* configure_data: default batch_names now use f-string formatting
  (``f'batch_{i}'``) instead of the literal string ``'batch_%d'``
  repeated len(datalist) times.
* configure_data: bad ATAC config now raises ValueError instead of
  calling exit() — exit() killed the Jupyter kernel with no traceback.
* download_file: accept str or pathlib.Path for dest_path. The
  signature was annotated str but the body called .name; both are
  now supported.
* VAE.forward: drop the bare ``try / except: logging.debug(...)``
  around the PoE call. The except swallowed real errors and the
  debug call had the wrong arg shape; the PoE itself is well-behaved
  for the supported input contract.
* model.py: drop unused ``import threading`` and the dead
  ``self.thread_lock = threading.Lock()`` (never acquired anywhere).
* .gitignore: add .claude/ and .nfs* to keep editor / NFS-stale-handle
  files out of working tree status.

tests/test_invariants.py adds 5 regression tests covering each of
the above plus the DDP sampler determinism fix from 857e8f5
(cross-rank disjoint indices, set_epoch actually changes ordering).
Each basics demo now exposes a small ``# === GPU configuration ===``
block at the top of cell 1 (``GPUS`` + ``STRATEGY`` + derived
``DEVICES``), matching the pattern introduced by 2b5494b ("rewrite
multi-GPU tutorial to match basics demos"). Switching from single-GPU
to multi-GPU now only requires editing those two values, with a comment
spelling out the ddp_notebook vs ddp script trade-off.

With the demos themselves carrying the multi-GPU instructions, the
standalone ``advanced/multi_gpu.rst`` tutorial is redundant — the
common failure modes it described (devices='gpu' typo, hard-coded
CUDA_VISIBLE_DEVICES='0', model.train vs L.Trainer split) are now
addressed inline where they would actually be encountered. Drop the
file and its toctree entry in advanced/index.rst.

demo3 also gains a small refinement in cell 7: z_c / z_u stay as
dense ndarray (they're 32-dim and 2-dim, sparsifying buys nothing),
while x_bc reconstructions remain csr (RNA/ATAC are ~85% zeros, so
csr saves real memory).
…e notes

* README:
  - Drop the standalone "Update: Load data from MuData" section. The
    from_mdata path is now a one-line "alternative input formats"
    callout linking to the docs, instead of a 50-line near-duplicate
    of the Quick Example.
  - Quick Example: correct the misleading "input should be an AnnData
    object" comment — configure_data_from_dir reads a directory of
    per-batch MTX matrices, not AnnData.
  - License badge: fix the broken link
    (github.com/labomics/midas/LICENSE was 404; now points to
    .../blob/main/LICENSE).

* Single-source the version. ``pyproject.toml`` is the sole
  authority; ``scmidas.__version__`` and Sphinx ``release`` both
  read it via ``importlib.metadata.version("scmidas")``. Removes
  the previous three-place duplication that drifted historically.

* docs/source/release.md: promote the "Unreleased" section to
  ``v0.1.18 (2026-05-02)`` and add entries for the new API
  hardening fixes, the test suite, the docs reorganisation, and
  the packaging change.

* Bump version 0.1.17 → 0.1.18.
…ick Example with API sketch

The README repeated "what is MIDAS" four times (title, tagline,
two-paragraph prose, Key Features) and surfaced the paper /
documentation links twice each. Trimming:

* Drop the two-paragraph "MIDAS is a powerful deep probabilistic
  framework..." block — the tagline above it and Key Features below
  it cover the same ground without the marketing register.
* Drop the standalone "Publication" bullet under the description;
  the Citation section already has the same link.
* Drop "For more detailed tutorials..." — the docs link is now in
  the Quick Start preamble where it actually fits.
* Tighten Key Features bullets — strip "powerful", "Seamlessly",
  "Accurately", "Effectively", "Advanced", "highly efficient";
  keep the concrete capability per bullet.

Quick Start is also reshaped:

* The previous "Quick Example" looked runnable but wasn't —
  ``'path/to/your/data'`` was a placeholder, ``predict()`` /
  ``get_emb_umap()`` depended on session state. Users who tried to
  copy-paste it would hit failures.
* Replaced with a 4-call API sketch using ``...`` placeholders so
  it's clearly not a script. The MuData alternative is now a
  comment in the first step instead of a duplicated block below.

Net: 113 lines → 95 lines, no information loss for the reader who
clicks through to the docs.
…n contributing

* Title: ``MIDAS: A Deep Generative Model for Mosaic Integration and
  Knowledge Transfer of Single-Cell Multimodal Data`` → ``MIDAS``.
  The tagline below already states what MIDAS does; the long subtitle
  was paper-style and made the H1 line wrap on narrow screens.
* Drop section-header emoji (✨🚀⚡📈📜🙌📝). Matches the convention
  of scvi-tools / scanpy and prints cleanly in non-emoji terminals.
* Logo width 900px → 450px so it doesn't get squashed in GitHub's
  mobile rendering (and isn't oversized on desktop either).
* Reproducibility section: drop the trailing slash on the branch URL
  and reword to one line.
* Citation:
  - Drop the duplicated plain-text human-readable line — the bibtex
    contains the same information.
  - Fix bibtex inaccuracies:
    * Author list now full (15 authors, no "and others"); previously
      truncated at 11 with et-al wildcard.
    * pages: ``1--12`` (placeholder) → ``1594--1605``.
    * Add volume (42), number (10), and doi.
    * publisher: ``Nature Publishing Group US New York`` (auto-tool
      output) → ``Nature Publishing Group``.
    * Wrap ``MIDAS`` in braces so bibtex preserves the casing.
* Contributing section: replace generic boilerplate with concrete
  guidance (open issue / branch from main / pytest passes / discuss
  non-trivial changes first).
* Drop ``Get started with MIDAS by setting up a conda environment.``
  preamble and the comment-numbered steps in the install block;
  three commands are self-explanatory.

Net: 95 lines → 92 lines, but readability up.
The previous trim to 450px was an over-correction. The source PNG
is 2626×535 (~5:1 banner) — at 450px the rendered height drops to
about 91px and the wordmark goes blurry. Mobile browsers already
auto-scale the image to fit the viewport, so the original 900px
was not actually being squashed; it just rendered smaller on
narrower screens, which is the desired behaviour.
…roject URLs

* CI: the test workflow now runs on Python 3.10, 3.11 and 3.12
  (matrix, fail-fast disabled). Previously only 3.12 was exercised,
  even though pyproject.toml declares ``requires-python = ">=3.10"``.
  This makes the declared support promise an actually-enforced one.

* PEP 561: add empty ``src/scmidas/py.typed`` marker and ship it via
  ``[tool.setuptools.package-data]`` so type checkers (mypy / pyright /
  pylance) honour scmidas's annotations instead of treating the
  package as untyped. Improves IDE jump-to-definition / hover for
  downstream users.

* README: add a CI status badge so users / reviewers can see at a
  glance whether ``main`` is green (matches scvi-tools / scanpy
  README convention).

* pyproject.toml ``[project.urls]``: add Documentation, Source, and
  Changelog entries. PyPI surfaces these as a "Project links"
  sidebar; the previous two URLs (Homepage + Issues) hid the docs
  link from the PyPI page despite the docs being well-developed.
…plates)

GitHub's Insights → Community Standards page treats these as the
baseline a "professional" repo is expected to surface, and they
match the file set scvi-tools / scanpy / numpy / pandas all ship.
None of them touch source code or runtime behaviour.

* CONTRIBUTING.md (root): concrete dev setup (clone + dev extras +
  pytest), bug-report what-to-include, PR workflow, when to file an
  issue first. Replaces the generic "We welcome contributions!"
  paragraph that was in the README.
* CHANGELOG.md (root): one-paragraph stub pointing to
  docs/source/release.md (the canonical changelog rendered on the
  docs site). Exists so the repo header surfaces a "CHANGELOG" link
  and tools that scan repo roots can find it.
* CODE_OF_CONDUCT.md (root): Contributor Covenant 2.1 verbatim
  from contributor-covenant.org, with the contact placeholder set
  to omicshub@outlook.com (the project maintainer address from
  pyproject.toml).
* .github/ISSUE_TEMPLATE/bug_report.md and feature_request.md:
  Markdown templates pre-populated with the fields that actually
  help diagnose MIDAS issues (scmidas/torch versions, single vs
  multi-GPU, full traceback in a code block). Preferred over an
  empty "Describe the issue" textarea.
* .github/ISSUE_TEMPLATE/config.yml: disables blank issues so users
  pick a template, and adds a Discussions contact link for
  questions that aren't bug reports.
* .github/PULL_REQUEST_TEMPLATE.md: short PR checklist (tests pass,
  docstrings updated, release notes touched, design discussed for
  non-trivial changes).
The previous ``torch>=2.5,<2.6`` cap was a defensive workaround for a
suspected Lightning DDP incompatibility on torch >= 2.6. That worry
has not held up in practice:

* The 0.1.18 hardening pass (notably the ``MyDistributedSampler``
  fix in 857e8f5) was developed and validated on torch 2.8.0 + cu128.
* A full 1000-epoch demo3 mosaic DDP run on 4× GPU with the new
  code was completed; the resulting UMAP and per-modality embeddings
  match the single-GPU baseline. No NCCL hangs, no watchdog timeouts.

Widen to ``torch>=2.5,<3`` (with matching ``torchvision>=0.20,<1`` /
``torchaudio>=2.5,<3``) so users on torch 2.6 / 2.7 / 2.8 no longer
have to manually override the pin. The major-version cap stays in
place so a hypothetical breaking torch 3.x release does not silently
land via a regular ``pip install --upgrade``.

CI continues to pin ``torch==2.5.1+cpu`` explicitly in the workflow
file, so this change does not alter what gets exercised in the matrix
runs — it only widens the surface ``pip install scmidas`` will accept
in the wild.
@zhen-he zhen-he changed the title 0.1.18 release: DDP fix + API hardening + docs polish 0.1.18: DDP fix + API hardening + repo polish + relax torch pin May 2, 2026
@zhen-he zhen-he merged commit c6d28b5 into main May 2, 2026
3 checks passed
@zhen-he zhen-he deleted the fix/ddp-mosaic-sampler branch May 2, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant